fix(ci): resolve CI failures, skip warm-start ANALYZE, add init timing logs#200
Merged
Conversation
- Detekt: fix MultipleEmitters in DeviceSetupWizard (wrap in Column), ComposableParamOrder in SectionBadge (modifier before onClick), ComplexCondition in SettingsDialog (extract to local val) - Bazel Android: add ktoml dep missing from androidMain BUILD.bazel - Wasm/JS: move js() calls to top-level functions (Kotlin/Wasm restriction), replace onLeft with Either.Left pattern match to avoid missing import - SQLDelight: sync generated sources (idx_pages_section_id in create()) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ktoml 0.7.1 generates WASM bytecode that fails binaryen wasm-opt validation during the production bundle optimization step. Move ktoml to jvmCommonMain + iosMain and introduce an expect/actual split: - jvmCommonMain/iosMain: actual uses ktoml for real TOML parsing - wasmJsMain: actual returns null (SectionManifestParser falls back to empty SectionManifest — no file system in the browser anyway) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…nManifestWriter SectionManifestWriter.kt (commonMain) referenced sectionToml directly, which was moved to jvmCommonMain in the ktoml WASM fix. Apply the same expect/actual split: JVM+iOS actuals delegate to ktoml, WASM stub throws (caught by the surrounding Either error handler). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tats ANALYZE blocks + ANALYZE pages ran unconditionally every startup (~50ms each on Android). Gate them on sqlite_stat1 having no entry for the table — the only case that needs explicit ANALYZE is the second startup after a fresh install (when the analyze_blocks migration ran on an empty table and wrote no stats). Warm starts now run 2× cheap SELECT instead. PRAGMA optimize continues to run unconditionally to handle stale stats for all other tables. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Logs ms elapsed at each major step (preFlightJob, createRepositorySet, UuidMigration, content migrations) so the overlay shows where init time is actually spent. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
JVM Load Benchmark (Desktop)Synthetic in-memory benchmark measuring load performance for the desktop (JVM) app.
Flamegraphs (this PR)**Allocation** — object allocation pressure (JDBC/SQLite churn)Alloc flamegraph not available CPU — method-level hotspots by on-CPU time CPU flamegraph not available Top allocation hotspots (this PR)`36.4%` byte[]_[k] `7.6%` java.lang.String_[k] `7.1%` java.util.LinkedHashMap$Entry_[k] `6.8%` int[]_[k] `3.5%` java.lang.StringBuilder_[k]Top CPU hotspots (this PR)`94.9%` /usr/lib/x86_64-linux-gnu/libc.so.6 `2%` /tmp/sqlite-3.51.3.0-3427415d-6fce-4c83-9f49-356c2bf6597f-libsqlitejdbc.so `0.7%` __libc_pwrite `0.5%` fsync `0.2%` pthread_cond_signal |
There was a problem hiding this comment.
Pull request overview
This PR addresses CI failures and platform build issues (notably Kotlin/Wasm), introduces platform-specific TOML parsing/encoding for section manifests, and improves startup performance/observability by skipping redundant SQLite ANALYZE work on warm starts and adding init timing logs.
Changes:
- Move section manifest TOML parsing/encoding behind
expect/actualso ktoml is only used on JVM/iOS, with WASM stubs. - Reduce warm-start startup work by skipping
ANALYZE blocks/pageswhensqlite_stat1already contains stats; add init elapsed-time logs inGraphManager. - Fix Kotlin/Wasm
js()interop placement and apply minor UI/compose refactors.
Reviewed changes
Copilot reviewed 13 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| kmp/src/wasmJsMain/kotlin/dev/stapler/stelekit/sync/WasmSectionSyncService.kt | Moves js() helpers to top-level for Kotlin/Wasm compatibility; replaces Arrow onLeft usage with explicit Either.Left check. |
| kmp/src/wasmJsMain/kotlin/dev/stapler/stelekit/sections/SectionManifestTomlDecoder.js.kt | WASM actuals: decoding returns null (parser falls back), encoding throws unsupported op. |
| kmp/src/jvmCommonMain/kotlin/dev/stapler/stelekit/sections/SectionManifestTomlDecoder.jvm.kt | JVM actuals using ktoml for decoding/encoding section manifest TOML. |
| kmp/src/iosMain/kotlin/dev/stapler/stelekit/sections/SectionManifestTomlDecoder.ios.kt | iOS actuals using ktoml for decoding/encoding section manifest TOML. |
| kmp/src/generated/sqldelight/dev/stapler/stelekit/db/SteleDatabaseQueries.kt | Generated SQLDelight changes (formatting/arg count expression). |
| kmp/src/generated/sqldelight/dev/stapler/stelekit/db/kmp/SteleDatabaseImpl.kt | Generated schema creation adds idx_pages_section_id. |
| kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/onboarding/DeviceSetupWizard.kt | Wraps mode selection/custom mode UI in Column containers. |
| kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/components/settings/SettingsDialog.kt | Refactors null-checks for Sections settings rendering; uses !! after consolidated condition. |
| kmp/src/commonMain/kotlin/dev/stapler/stelekit/ui/components/SectionBadge.kt | Reorders parameters to put modifier before onClick. |
| kmp/src/commonMain/kotlin/dev/stapler/stelekit/sections/SectionManifestWriter.kt | Routes TOML encoding through encodeSectionManifestToml expect/actual. |
| kmp/src/commonMain/kotlin/dev/stapler/stelekit/sections/SectionManifestParser.kt | Routes TOML decoding through decodeSectionManifestToml expect/actual with WASM fallback to empty manifest. |
| kmp/src/commonMain/kotlin/dev/stapler/stelekit/db/MigrationRunner.kt | Skips ANALYZE when stats already exist; adds hasStats helper. |
| kmp/src/commonMain/kotlin/dev/stapler/stelekit/db/GraphManager.kt | Adds elapsed-time log lines across switchGraph init sequence. |
| kmp/src/androidMain/kotlin/BUILD.bazel | Adds ktoml dependency for Android Bazel build. |
| kmp/build.gradle.kts | Moves ktoml dependency out of commonMain into jvmCommonMain and iosMain. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Android Load BenchmarkInstrumented benchmark on an API 30 x86_64 emulator — 500-page synthetic graph. Comparing Graph Load
Interactive Write Latency (during Phase 3)
SAF I/O Overhead (ContentProvider vs direct File read)Measures Binder IPC cost added by ContentResolver per readFile() call.
|
…ce cold-start shadow timeout - RepositoryFactory: open telemetry DB in a parallel coroutine alongside main DB init (write + read driver opens). No data dependency between them; on a real device each open is 50–250ms, so this reduces createRepositorySet wall-time by that amount. runBlocking join is bounded — telemetry typically completes before the main DB lazy. - GraphManager: emit a "db.init" span (startMs → after content migrations) using the repo set's spanEmitter. Makes driver-open + migration latency visible in the Spans tab and eligible for an SLO threshold; previously this entire path was uninstrumented. - GraphLoader: reduce SHADOW_STARTUP_TIMEOUT_MS from 2000ms → 500ms. Failure is non-fatal and SAF cursor queries complete well under 500ms; the old 2s ceiling was the worst-case TTI penalty on cold starts with a slow SAF provider. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…epositorySet
- MigrationRunner.hasStats: add catch(e: CancellationException) { throw e } to
match the structured-concurrency contract used everywhere else in the file;
without it, coroutine cancellation on WASM (single-threaded dispatcher) would
be swallowed and trigger spurious ANALYZE work on a cancelled scope
- RepositoryFactory.createRepositorySet: promote to suspend fun and replace
runBlocking { telemetryDeferred.await() } with direct await(); removes the
hidden blocking call that could deadlock on single-threaded dispatchers (WASM)
and violates the method's implied non-blocking contract; all 15+ call sites
are already inside runBlocking/runTest so no callers need changing
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Address two copilot review comments on MigrationRunner: - Update the comment block above the conditional ANALYZE calls to correctly describe the new warm-start skip behavior; removes the old "unconditionally" wording that no longer applied - Parameterize the hasStats SQL query (? + bindString) instead of string interpolation; matches the parameterized convention used elsewhere in the file and prevents accidental injection if the private helper is ever reused Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n blocks Two catch(_: Exception) blocks in RepositoryFactory.createRepositorySet were swallowing CancellationException. Detekt flagged both. Add the standard guard to match the pattern used throughout the codebase. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
encodeSectionManifestTomlexpect/actual forSectionManifestWriterANALYZE blocks/pageson warm starts whensqlite_stat1already has stats — saves ~100ms per startup on Android by checkingsqlite_stat1before running ANALYZE instead of running it unconditionally every launchswitchGraphinit sequence so the on-screen overlay shows exactly where init time is spentTest plan
bazel test //kmp:business_testsbazel test //kmp:jvm_testsinit[Xms]: createRepositorySet donewhere X is measurably lower than beforesqlite_stat1has no entry for blocks/pages)🤖 Generated with Claude Code